Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

don't splice from files into pipes in io::copy #108283

Merged
merged 2 commits into from
Apr 13, 2023

Conversation

the8472
Copy link
Member

@the8472 the8472 commented Feb 20, 2023

This fixes potential data ordering issue where a write performed after a copy operation could become visible in the copy even though it signaled completion.

I assumed that by not setting SPLICE_F_MOVE we would be safe and the kernel would do a copy in kernel space and we could avoid the read-write syscall and copy-to/from-userspace costs. But apparently that flag only makes a difference when splicing from a pipe, but not when splicing into it.

Context: https://lkml.org/lkml/2023/2/9/673

@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2023

r? @m-ou-se

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 20, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 20, 2023

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

  • Stabilizing library features
  • Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
  • Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
  • Changing public documentation in ways that create new stability guarantees
  • Changing observable runtime behavior of library APIs

@ChrisDenton
Copy link
Member

Reading through that thread it makes sense but.. that seems unfortunate? This could be a soundness issue, right?

Does this need a test?

@the8472
Copy link
Member Author

the8472 commented Mar 27, 2023

I don't think rust code can see bytes in memory change under its nose, so there's no soundness issue with non-deterministic bytes.
But it could be an ordering issue leading to logic bugs where one copies data and then modifies the source and the modified version becomes visible on the other side instead of the expected original.

Thinking about it, this shouldn't affect sockets because their buffers can't be modified, so splicing from a socket to a pipe could still be ok. The linked mail also describes this as case 1.

And yeah, I'll add a test, it's subtle enough.

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 27, 2023
@the8472
Copy link
Member Author

the8472 commented Apr 3, 2023

Oh bloody, sendfile has the same issue:

   If out_fd refers to a socket or pipe with zero-copy support,
   callers must ensure the transferred portions of the file referred
   to by in_fd remain unmodified until the reader on the other end
   of out_fd has consumed the transferred data.

Removing that optimization for the File -> Socket case removes the main benefit of sendfile that people have asked for. 🙁

@the8472 the8472 force-pushed the remove-splice-into-pipe branch from b4fa02c to 171ccb5 Compare April 3, 2023 19:11
@the8472
Copy link
Member Author

the8472 commented Apr 3, 2023

@rustbot ready

added a test and checked that it fails without the fix.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 3, 2023
@ChrisDenton
Copy link
Member

Thank you for investigating this.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 12, 2023

📌 Commit 171ccb5 has been approved by ChrisDenton

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 12, 2023
@bors
Copy link
Contributor

bors commented Apr 13, 2023

⌛ Testing commit 171ccb5 with merge 4f418e71c2c591d3f3b2811ddf62cebac5a3b7ba...

@bors
Copy link
Contributor

bors commented Apr 13, 2023

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 13, 2023
@ChrisDenton
Copy link
Member

Hm, STATUS_ILLEGAL_INSTRUCTION on i686-pc-windows-msvc looks a bit worrying but it should not have anything to do with this PR.

@bors retry

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 13, 2023
@bors
Copy link
Contributor

bors commented Apr 13, 2023

⌛ Testing commit 171ccb5 with merge a29dada...

@bors
Copy link
Contributor

bors commented Apr 13, 2023

☀️ Test successful - checks-actions
Approved by: ChrisDenton
Pushing a29dada to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 13, 2023
@bors bors merged commit a29dada into rust-lang:master Apr 13, 2023
@rustbot rustbot added this to the 1.70.0 milestone Apr 13, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (a29dada): comparison URL.

Overall result: ❌ regressions - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
1.4% [1.4%, 1.4%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
1.6% [1.6%, 1.6%] 1
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.7%, 0.6%] 2

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.6% [0.6%, 0.6%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.1% [-0.7%, 0.6%] 2

@rust-log-analyzer
Copy link
Collaborator

The job i686-msvc-1 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
   Compiling sourcegen v0.0.0 (C:\a\rust\rust\src\tools\rust-analyzer\crates\sourcegen)
[RUSTC-TIMING] sourcegen test:false 3.667
   Compiling proc-macro-srv-cli v0.0.0 (C:\a\rust\rust\src\tools\rust-analyzer\crates\proc-macro-srv-cli)
[RUSTC-TIMING] rust_analyzer_proc_macro_srv test:false 0.976
Assertion failed: !KeyInfoT::isEqual(Val, EmptyKey) && !KeyInfoT::isEqual(Val, TombstoneKey) && "Empty/Tombstone value shouldn't be inserted into map!", file C:\a\rust\rust\src\llvm-project\llvm\include\llvm/ADT/DenseMap.h, line 624
error: could not compile `hir-ty`

Caused by:
Caused by:
  process didn't exit successfully: `C:\a\rust\rust\build\bootstrap\debug\rustc --crate-name hir_ty --edition=2021 crates\hir-ty\src\lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C embed-bitcode=no -C debuginfo=0 -C debug-assertions=on -Zunstable-options --check-cfg values(feature) --check-cfg names() --check-cfg values() -C metadata=91277913358a0422 -C extra-filename=-91277913358a0422 --out-dir C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps --target i686-pc-windows-msvc -L dependency=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps -L dependency=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\release\deps --extern arrayvec=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libarrayvec-9d6091a37119fcba.rmeta --extern base_db=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libbase_db-f10262e2745c3ae3.rmeta --extern bitflags=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libbitflags-dee732d3861c88b9.rmeta --extern chalk_derive=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\release\deps\chalk_derive-74fe33590d67bb02.dll --extern chalk_ir=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libchalk_ir-c1e2d9a03988523e.rmeta --extern chalk_recursive=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libchalk_recursive-ff61d111d47ed187.rmeta --extern chalk_solve=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libchalk_solve-8753f40861d45017.rmeta --extern cov_mark=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libcov_mark-cb482023ffa296a4.rmeta --extern either=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libeither-52470f2aa7c714f1.rmeta --extern ena=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libena-a3265d6f37d7312f.rmeta --extern hir_def=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libhir_def-68051d7002233a5e.rmeta --extern hir_expand=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libhir_expand-066e10b1cf041501.rmeta --extern rustc_index=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libhkalbasi_rustc_ap_rustc_index-50488986d1e2acaa.rmeta --extern intern=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libintern-5ff99a8a902b6291.rmeta --extern itertools=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libitertools-53668c30ac2c4538.rmeta --extern la_arena=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libla_arena-55a88d685f7b2b4c.rmeta --extern limit=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\liblimit-ac07f608f496cd98.rmeta --extern once_cell=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libonce_cell-c4fab29fec5d0f5d.rmeta --extern profile=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libprofile-d2f6d4b9b0075f24.rmeta --extern rustc_hash=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\librustc_hash-9951669e3cef155d.rmeta --extern scoped_tls=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libscoped_tls-130876785c1f2e9c.rmeta --extern smallvec=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libsmallvec-a756a535f31976a3.rmeta --extern stdx=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libstdx-99e203d50d6741af.rmeta --extern syntax=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libsyntax-0f3d1f69b6409078.rmeta --extern tracing=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libtracing-268a7a2d20d16c24.rmeta --extern typed_arena=C:\a\rust\rust\build\i686-pc-windows-msvc\stage2-tools\i686-pc-windows-msvc\release\deps\libtyped_arena-757c6870dba4e216.rmeta -Csymbol-mangling-version=v0 -Zunstable-options --check-cfg=values(bootstrap) -Zmacro-backtrace -Csplit-debuginfo=packed -Ctarget-feature=+crt-static -Z binary-dep-depinfo -L native=C:\Users\runneradmin\.cargo\registry\src\index.crates.io-1cd66030c949c28d\windows_i686_msvc-0.42.1\lib` (exit code: 0xc000001d, STATUS_ILLEGAL_INSTRUCTION)
[RUSTC-TIMING] xtask test:false 23.027
[RUSTC-TIMING] hir_def test:false 265.730
[RUSTC-TIMING] ide_assists test:false 180.159
[RUSTC-TIMING] project_model test:false 72.604
[RUSTC-TIMING] project_model test:false 72.604
[RUSTC-TIMING] ide test:false 128.652
[RUSTC-TIMING] rust_analyzer test:false 85.970
Build completed unsuccessfully in 1:02:02
make: *** [Makefile:68: ci-subset-1] Error 1

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Jun 3, 2023
Pkgsrc changes:
 * Adjust patches and cargo checksums to new versions.
 * Adjust to not cross-build to 8.0, due to LLVM using c++17,
   so adjust USE_LANGUAGES.

Upstream changes:

Version 1.70.0 (2023-06-01)
==========================

Language
--------
- [Relax ordering rules for `asm!` operands]
  (rust-lang/rust#105798)
- [Properly allow macro expanded `format_args` invocations to uses
  captures] (rust-lang/rust#106505)
- [Lint ambiguous glob re-exports]
  (rust-lang/rust#107880)
- [Perform const and unsafe checking for expressions in `let _ =
  expr` position.]
  (rust-lang/rust#102256)

Compiler
--------
- [Extend -Cdebuginfo with new options and named aliases]
  (rust-lang/rust#109808)
  This provides a smaller version of debuginfo for cases that only
  need line number information (`-Cdebuginfo=line-tables-only`),
  which may eventually become the default for `-Cdebuginfo=1`.
- [Make `unused_allocation` lint against `Box::new` too]
  (rust-lang/rust#104363)
- [Detect uninhabited types early in const eval]
  (rust-lang/rust#109435)
- [Switch to LLD as default linker for {arm,thumb}v4t-none-eabi]
  (rust-lang/rust#109721)
- [Add tier 3 target `loongarch64-unknown-linux-gnu`]
  (rust-lang/rust#96971)
- [Add tier 3 target for `i586-pc-nto-qnx700` (QNX Neutrino RTOS,
  version 7.0)] (rust-lang/rust#109173),
- [Insert alignment checks for pointer dereferences as debug assertions]
  (rust-lang/rust#98112)
  This catches undefined behavior at runtime, and may cause existing
  code to fail.

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Document NonZeroXxx layout guarantees]
  (rust-lang/rust#94786)
- [Windows: make `Command` prefer non-verbatim paths]
  (rust-lang/rust#96391)
- [Implement Default for some alloc/core iterators]
  (rust-lang/rust#99929)
- [Fix handling of trailing bare CR in str::lines]
  (rust-lang/rust#100311)
- [allow negative numeric literals in `concat!`]
  (rust-lang/rust#106844)
- [Add documentation about the memory layout of `Cell`]
  (rust-lang/rust#106921)
- [Use `partial_cmp` to implement tuple `lt`/`le`/`ge`/`gt`]
  (rust-lang/rust#108157)
- [Stabilize `atomic_as_ptr`]
  (rust-lang/rust#108419)
- [Stabilize `nonnull_slice_from_raw_parts`]
  (rust-lang/rust#97506)
- [Partial stabilization of `once_cell`]
  (rust-lang/rust#105587)
- [Stabilize `nonzero_min_max`]
  (rust-lang/rust#106633)
- [Flatten/inline format_args!() and (string and int) literal
  arguments into format_args!()]
  (rust-lang/rust#106824)
- [Stabilize movbe target feature]
  (rust-lang/rust#107711)
- [don't splice from files into pipes in io::copy]
  (rust-lang/rust#108283)
- [Add a builtin unstable `FnPtr` trait that is implemented for
  all function pointers]
  (rust-lang/rust#108080)
  This extends `Debug`, `Pointer`, `Hash`, `PartialEq`, `Eq`,
  `PartialOrd`, and `Ord` implementations for function pointers
  with all ABIs.

Stabilized APIs
---------------

- [`NonZero*::MIN/MAX`]
  (https://doc.rust-lang.org/stable/std/num/struct.NonZeroI8.html#associatedconstant.MIN)
- [`BinaryHeap::retain`]
  (https://doc.rust-lang.org/stable/std/collections/struct.BinaryHeap.html#method.retain)
- [`Default for std::collections::binary_heap::IntoIter`]
  (https://doc.rust-lang.org/stable/std/collections/binary_heap/struct.IntoIter.html)
- [`Default for std::collections::btree_map::{IntoIter, Iter, IterMut}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoIter.html)
- [`Default for std::collections::btree_map::{IntoKeys, Keys}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoKeys.html)
- [`Default for std::collections::btree_map::{IntoValues, Values}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoKeys.html)
- [`Default for std::collections::btree_map::Range`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.Range.html)
- [`Default for std::collections::btree_set::{IntoIter, Iter}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_set/struct.IntoIter.html)
- [`Default for std::collections::btree_set::Range`]
  (https://doc.rust-lang.org/stable/std/collections/btree_set/struct.Range.html)
- [`Default for std::collections::linked_list::{IntoIter, Iter, IterMut}`]
  (https://doc.rust-lang.org/stable/alloc/collections/linked_list/struct.IntoIter.html)
- [`Default for std::vec::IntoIter`]
  (https://doc.rust-lang.org/stable/alloc/vec/struct.IntoIter.html#impl-Default-for-IntoIter%3CT,+A%3E)
- [`Default for std::iter::Chain`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Chain.html)
- [`Default for std::iter::Cloned`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Cloned.html)
- [`Default for std::iter::Copied`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Copied.html)
- [`Default for std::iter::Enumerate`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Enumerate.html)
- [`Default for std::iter::Flatten`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Flatten.html)
- [`Default for std::iter::Fuse`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Fuse.html)
- [`Default for std::iter::Rev`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Rev.html)
- [`Default for std::slice::Iter`]
  (https://doc.rust-lang.org/stable/std/slice/struct.Iter.html)
- [`Default for std::slice::IterMut`]
  (https://doc.rust-lang.org/stable/std/slice/struct.IterMut.html)
- [`Rc::into_inner`]
  (https://doc.rust-lang.org/stable/alloc/rc/struct.Rc.html#method.into_inner)
- [`Arc::into_inner`]
  (https://doc.rust-lang.org/stable/alloc/sync/struct.Arc.html#method.into_inner)
- [`std::cell::OnceCell`]
  (https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html)
- [`Option::is_some_and`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.is_some_and)
- [`NonNull::slice_from_raw_parts`]
  (https://doc.rust-lang.org/stable/std/ptr/struct.NonNull.html#method.slice_from_raw_parts)
- [`Result::is_ok_and`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.is_ok_and)
- [`Result::is_err_and`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.is_err_and)
- [`std::sync::atomic::Atomic*::as_ptr`]
  (https://doc.rust-lang.org/stable/std/sync/atomic/struct.AtomicU8.html#method.as_ptr)
- [`std::io::IsTerminal`]
  (https://doc.rust-lang.org/stable/std/io/trait.IsTerminal.html)
- [`std::os::linux::net::SocketAddrExt`]
  (https://doc.rust-lang.org/stable/std/os/linux/net/trait.SocketAddrExt.html)
- [`std::os::unix::net::UnixDatagram::bind_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.bind_addr)
- [`std::os::unix::net::UnixDatagram::connect_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.connect_addr)
- [`std::os::unix::net::UnixDatagram::send_to_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.send_to_addr)
- [`std::os::unix::net::UnixListener::bind_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixListener.html#method.bind_addr)
- [`std::path::Path::as_mut_os_str`]
  (https://doc.rust-lang.org/stable/std/path/struct.Path.html#method.as_mut_os_str)
- [`std::sync::OnceLock`]
  (https://doc.rust-lang.org/stable/std/sync/struct.OnceLock.html)

Cargo
-----

- [Add `CARGO_PKG_README`]
  (rust-lang/cargo#11645)
- [Make `sparse` the default protocol for crates.io]
  (rust-lang/cargo#11791)
- [Accurately show status when downgrading dependencies]
  (rust-lang/cargo#11839)
- [Use registry.default for login/logout]
  (rust-lang/cargo#11949)
- [Stabilize `cargo logout`]
  (rust-lang/cargo#11950)

Misc
----

- [Stabilize rustdoc `--test-run-directory`]
  (rust-lang/rust#103682)

Compatibility Notes
-------------------

- [Prevent stable `libtest` from supporting `-Zunstable-options`]
  (rust-lang/rust#109044)
- [Perform const and unsafe checking for expressions in `let _ =
  expr` position.] (rust-lang/rust#102256)
- [WebAssembly targets enable `sign-ext` and `mutable-globals`
  features in codegen] (rust-lang/rust#109807)
  This may cause incompatibility with older execution environments.
- [Insert alignment checks for pointer dereferences as debug assertions]
  (rust-lang/rust#98112)
  This catches undefined behavior at runtime, and may cause existing
  code to fail.

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Upgrade to LLVM 16]
  (rust-lang/rust#109474)
- [Use SipHash-1-3 instead of SipHash-2-4 for StableHasher]
  (rust-lang/rust#107925)
@the8472 the8472 mentioned this pull request Jul 10, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jul 10, 2023
Pkgsrc changes:
 * Adjust patches and cargo checksums to new versions.
 * Add support for NetBSD/riscv64.

Upstream changes:

Version 1.70.0 (2023-06-01)
==========================

Language
--------
- [Relax ordering rules for `asm!` operands]
  (rust-lang/rust#105798)
- [Properly allow macro expanded `format_args` invocations to uses captures]
  (rust-lang/rust#106505)
- [Lint ambiguous glob re-exports]
  (rust-lang/rust#107880)
- [Perform const and unsafe checking for expressions in `let _ =
  expr` position.]
  (rust-lang/rust#102256)

Compiler
--------
- [Extend -Cdebuginfo with new options and named aliases]
  (rust-lang/rust#109808)
  This provides a smaller version of debuginfo for cases that only
  need line number information (`-Cdebuginfo=line-tables-only`),
  which may eventually become the default for `-Cdebuginfo=1`.
- [Make `unused_allocation` lint against `Box::new` too]
  (rust-lang/rust#104363)
- [Detect uninhabited types early in const eval]
  (rust-lang/rust#109435)
- [Switch to LLD as default linker for {arm,thumb}v4t-none-eabi]
  (rust-lang/rust#109721)
- [Add tier 3 target `loongarch64-unknown-linux-gnu`]
  (rust-lang/rust#96971)
- [Add tier 3 target for `i586-pc-nto-qnx700`(QNX Neutrino RTOS, version 7.0)]
  (rust-lang/rust#109173),
- [Insert alignment checks for pointer dereferences as debug assertions]
  (rust-lang/rust#98112)
  This catches undefined behavior at runtime, and may cause existing
  code to fail.

Refer to Rust's [platform support page][platform-support-doc]
for more information on Rust's tiered platform support.

Libraries
---------
- [Document NonZeroXxx layout guarantees]
  (rust-lang/rust#94786)
- [Windows: make `Command` prefer non-verbatim paths]
  (rust-lang/rust#96391)
- [Implement Default for some alloc/core iterators]
  (rust-lang/rust#99929)
- [Fix handling of trailing bare CR in str::lines]
  (rust-lang/rust#100311)
- [allow negative numeric literals in `concat!`]
  (rust-lang/rust#106844)
- [Add documentation about the memory layout of `Cell`]
  (rust-lang/rust#106921)
- [Use `partial_cmp` to implement tuple `lt`/`le`/`ge`/`gt`]
  (rust-lang/rust#108157)
- [Stabilize `atomic_as_ptr`]
  (rust-lang/rust#108419)
- [Stabilize `nonnull_slice_from_raw_parts`]
  (rust-lang/rust#97506)
- [Partial stabilization of `once_cell`]
  (rust-lang/rust#105587)
- [Stabilize `nonzero_min_max`]
  (rust-lang/rust#106633)
- [Flatten/inline format_args!() and (string and int) literal
  arguments into format_args!()]
  (rust-lang/rust#106824)
- [Stabilize movbe target feature]
  (rust-lang/rust#107711)
- [don't splice from files into pipes in io::copy]
  (rust-lang/rust#108283)
- [Add a builtin unstable `FnPtr` trait that is implemented for
  all function pointers]
  (rust-lang/rust#108080)
  This extends `Debug`, `Pointer`, `Hash`, `PartialEq`, `Eq`,
  `PartialOrd`, and `Ord` implementations for function pointers
  with all ABIs.


Stabilized APIs
---------------

- [`NonZero*::MIN/MAX`]
  (https://doc.rust-lang.org/stable/std/num/struct.NonZeroI8.html#associatedconstant.MIN)
- [`BinaryHeap::retain`]
  (https://doc.rust-lang.org/stable/std/collections/struct.BinaryHeap.html#method.retain)
- [`Default for std::collections::binary_heap::IntoIter`]
  (https://doc.rust-lang.org/stable/std/collections/binary_heap/struct.IntoIter.html)
- [`Default for std::collections::btree_map::{IntoIter, Iter, IterMut}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoIter.html)
- [`Default for std::collections::btree_map::{IntoKeys, Keys}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoKeys.html)
- [`Default for std::collections::btree_map::{IntoValues, Values}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.IntoKeys.html)
- [`Default for std::collections::btree_map::Range`]
  (https://doc.rust-lang.org/stable/std/collections/btree_map/struct.Range.html)
- [`Default for std::collections::btree_set::{IntoIter, Iter}`]
  (https://doc.rust-lang.org/stable/std/collections/btree_set/struct.IntoIter.html)
- [`Default for std::collections::btree_set::Range`]
  (https://doc.rust-lang.org/stable/std/collections/btree_set/struct.Range.html)
- [`Default for std::collections::linked_list::{IntoIter, Iter, IterMut}`]
  (https://doc.rust-lang.org/stable/alloc/collections/linked_list/struct.IntoIter.html)
- [`Default for std::vec::IntoIter`]
  (https://doc.rust-lang.org/stable/alloc/vec/struct.IntoIter.html#impl-Default-for-IntoIter%3CT,+A%3E)
- [`Default for std::iter::Chain`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Chain.html)
- [`Default for std::iter::Cloned`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Cloned.html)
- [`Default for std::iter::Copied`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Copied.html)
- [`Default for std::iter::Enumerate`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Enumerate.html)
- [`Default for std::iter::Flatten`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Flatten.html)
- [`Default for std::iter::Fuse`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Fuse.html)
- [`Default for std::iter::Rev`]
  (https://doc.rust-lang.org/stable/std/iter/struct.Rev.html)
- [`Default for std::slice::Iter`]
  (https://doc.rust-lang.org/stable/std/slice/struct.Iter.html)
- [`Default for std::slice::IterMut`]
  (https://doc.rust-lang.org/stable/std/slice/struct.IterMut.html)
- [`Rc::into_inner`]
  (https://doc.rust-lang.org/stable/alloc/rc/struct.Rc.html#method.into_inner)
- [`Arc::into_inner`]
  (https://doc.rust-lang.org/stable/alloc/sync/struct.Arc.html#method.into_inner)
- [`std::cell::OnceCell`]
  (https://doc.rust-lang.org/stable/std/cell/struct.OnceCell.html)
- [`Option::is_some_and`]
  (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.is_some_and)
- [`NonNull::slice_from_raw_parts`]
  (https://doc.rust-lang.org/stable/std/ptr/struct.NonNull.html#method.slice_from_raw_parts)
- [`Result::is_ok_and`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.is_ok_and)
- [`Result::is_err_and`]
  (https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.is_err_and)
- [`std::sync::atomic::Atomic*::as_ptr`]
  (https://doc.rust-lang.org/stable/std/sync/atomic/struct.AtomicU8.html#method.as_ptr)
- [`std::io::IsTerminal`]
  (https://doc.rust-lang.org/stable/std/io/trait.IsTerminal.html)
- [`std::os::linux::net::SocketAddrExt`]
  (https://doc.rust-lang.org/stable/std/os/linux/net/trait.SocketAddrExt.html)
- [`std::os::unix::net::UnixDatagram::bind_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.bind_addr)
- [`std::os::unix::net::UnixDatagram::connect_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.connect_addr)
- [`std::os::unix::net::UnixDatagram::send_to_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixDatagram.html#method.send_to_addr)
- [`std::os::unix::net::UnixListener::bind_addr`]
  (https://doc.rust-lang.org/stable/std/os/unix/net/struct.UnixListener.html#method.bind_addr)
- [`std::path::Path::as_mut_os_str`]
  (https://doc.rust-lang.org/stable/std/path/struct.Path.html#method.as_mut_os_str)
- [`std::sync::OnceLock`]
  (https://doc.rust-lang.org/stable/std/sync/struct.OnceLock.html)

Cargo
-----

- [Add `CARGO_PKG_README`]
  (rust-lang/cargo#11645)
- [Make `sparse` the default protocol for crates.io]
  (rust-lang/cargo#11791)
- [Accurately show status when downgrading dependencies]
  (rust-lang/cargo#11839)
- [Use registry.default for login/logout]
  (rust-lang/cargo#11949)
- [Stabilize `cargo logout`]
  (rust-lang/cargo#11950)

Misc
----

- [Stabilize rustdoc `--test-run-directory`]
  (rust-lang/rust#103682)

Compatibility Notes
-------------------

- [Prevent stable `libtest` from supporting `-Zunstable-options`]
  (rust-lang/rust#109044)
- [Perform const and unsafe checking for expressions in `let _ =
  expr` position.]
  (rust-lang/rust#102256)
- [WebAssembly targets enable `sign-ext` and `mutable-globals`
  features in codegen]
  (rust-lang/rust#109807)
  This may cause incompatibility with older execution environments.
- [Insert alignment checks for pointer dereferences as debug assertions]
  (rust-lang/rust#98112)
  This catches undefined behavior at runtime, and may cause existing
  code to fail.

Internal Changes
----------------

These changes do not affect any public interfaces of Rust, but they represent
significant improvements to the performance or internals of rustc and related
tools.

- [Upgrade to LLVM 16]
  (rust-lang/rust#109474)
- [Use SipHash-1-3 instead of SipHash-2-4 for StableHasher]
  (rust-lang/rust#107925)
@NobodyXu
Copy link
Contributor

For optimization reverted here, I think we could add it back with an intermediate pipe?

I.e. first splice to an intermediate pipe with F_MOVE, then splice to the destination pipe without F_MOVE?

The intermediate pipe can be cached and reused, given that we can make sure there's no panicking within io::copy

@the8472
Copy link
Member Author

the8472 commented Jul 28, 2024

If that works at all it'd still negate some of the advantages of splice. More syscalls, more copying. It still might provide some more throughput because pipes would be a larger buffer than our 8k on-stack buffer. Maybe worth a try.

In theory using sendfile/splice from read-only files might be ok, but then the question is in which state the file or file descriptor must be so that we could consider it sufficiently read-only.
Another theoretical option would be to wait for the socket send buffer to drain. Which would be fine for sending huge files, but not when using io::copy to send some chunk of a smaller file.

@NobodyXu
Copy link
Contributor

Maybe worth a try.

Thanks, I'd try opening a PR for this.

but then the question is in which state the file or file descriptor must be so that we could consider it sufficiently read-only.

Unless it's /dev/zero or on an immutable system, I suppose the file can be changed at anytime.

I thought about if CoW filesystem is immune to this, but then I think that's impossible because the mmap abstraction provides by the kernel guarantees it will see updates.

That reminds me, that on CoW fs, copy_file_range can be very cheap as it requires no copy only reflink, we could also create a O_TMPFILE and use it to achieve zero-copy.

Another theoretical option would be to wait for the socket send buffer to drain.

I think the best fix would be for the Linux kernel to fix this, if they would provide a flag to opt-in to CoW behavior.

It'd also be great if mmap has such flags, currently it says with MMAP_PRIVATE read-only mmap, it is unspecified if changes to the original file would affect the mmap.

@NobodyXu
Copy link
Contributor

@the8472 I think sealed files can be used here, if the file has flag F_SEAL_WRITE (prevent modifying of existing contents, shrinking and appending is allowed) and F_SEAL_SEAL (not changing of seals).

@NobodyXu
Copy link
Contributor

NobodyXu commented Jul 28, 2024

So to sum up, I think we can add back optimization via:

  • if the file is on squashfs/erofs, then it's immutable (detectable via statfs
  • if the is sealed with F_SEAL_WRITE (prevent modifying of existing contents, shrinking and appending is allowed) and F_SEAL_SEAL (not changing of seals), detectable via fcntl(fd, F_GET_SEALS)
  • if it's on read-only btrfs subvolume (e.g. snapshot), likewise we can do the same for zfs, nilfs, bcachefs
  • If the file is /dev/zero, or from other pseudo fs (e.g. procfs, sysfe), then I think we can assume it's immutable

Otherwise, we'd fallback to:

  • read + write if the file is really small
  • splice to an intermediate pipe with F_MOVE and then splice to the pipe without F_MOVE if the file is more than 1-2 pages
  • if the fs supports reflink and file is really large, then we could create a O_TMPFILE on the system, copy_file_range to it and then splice it

Also, if the file is a named fifo, then just a splice without F_MOVE would work fine.

@NobodyXu
Copy link
Contributor

@the8472 opened #128300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants